Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to run the latest versions of webid, crud and wac tests #1756

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

michielbdejong
Copy link
Member

@michielbdejong michielbdejong commented Feb 9, 2024

I got this passing:

  • git checkout 73e73c8
  • webid-provider-tests v2.0.3 -> 36 passed, 36 total
  • solid-crud-tests v6.0.0 -> 4 skipped, 57 passed, 61 total
  • web-access-control-tests v7.1.0 -> 2 skipped, 89 passed, 91 total

(FIXME, intermittent in the WAC tests: 1 failed, 2 skipped, 88 passed, 91 total)

To do:

  • use latest main branch of NSS
  • use webid-provider-tests 2.1.1
  • use solid-crud-tests v7.0.6

@michielbdejong michielbdejong marked this pull request as draft February 9, 2024 09:47
@michielbdejong michielbdejong marked this pull request as ready for review February 9, 2024 10:09
@michielbdejong
Copy link
Member Author

@bourgeoa it seems the behaviour of NSS has diverged from what the WAC tests prescribe. I know there was a discussion about this in the matrix chat but didn't follow the whole conversation. What is the current status? Is there now still a difference in how we interpret the WAC spec, that causes this diversion? If so I'll tag the 5 failing tests as disputed.

@bourgeoa
Copy link
Member

bourgeoa commented Feb 11, 2024

@michielbdejong

FIXME, intermittent in the WAC tests: 1 failed, 2 skipped, 88 passed, 91 total

This is a real pain. I always :

  • check locally that all tests passes
  • retry the CI, and sometimes it works

The main environment difference is the node version :

  • 18 locally ( I removed 16 because I have success on CI on 2 successive run
  • 21 on the CI

I tried node 21 locally and tests passes.
I just added IPv6 localhosts in /etc/hosts :

::1 localhost
::1 alice.localhost
::1 bob.localhost

Could it be due to :
A worker process has failed to exit gracefully and has been force exited. This is likely caused by tests leaking due to improper teardown. Try running with --detectOpenHandles to find leaks.

The message disappear if I add --detectOpenHandles to jest

@bourgeoa
Copy link
Member

bourgeoa commented Feb 11, 2024

@michielbdejong

it seems the behaviour of NSS has diverged from what the WAC tests prescribe

is Append allowed on PUT and PATCH ?

the following discussion solid/web-access-control-spec#122 concluded that there was no need to change the spec :

--> Append is allowed for PUT for create (the resource do not exist)
This has since been implemented in CSS v7.0.3, was not tested and is still not tested in the specifications tests see solid-contrib/specification-tests#111

--> Append is also allowed on C/ for PATCH to create

Following your request some rewrite to the spec has been made as a clarification https://github.com/solid/web-access-control-spec/pull/128/files

NSS is now passing all specification-tests.
you can run these on https://solidcommunity.net:8443 which is today aligned with NSS main

bourgeoa@DESKTOP-PIRJOCS:/mnt/d/github/specification-tests$ cat nss.env
quarkus.log.category."org.solid.testharness.http.Client".level=DEBUG

USERS_ALICE_WEBID=https://alice.solidcommunity.net:8443/profile/card#me
USERS_BOB_WEBID=https://bob.solidcommunity.net:8443/profile/card#me

LOGIN_ENDPOINT=https://solidcommunity.net:8443/login/password
USERS_ALICE_USERNAME=alice
USERS_ALICE_PASSWORD=123
USERS_ALICE_IDP=https://solidcommunity.net:8443/
USERS_BOB_USERNAME=bob
USERS_BOB_PASSWORD=123
USERS_BOB_IDP=https://solidcommunity.net:8443/

bourgeoa@DESKTOP-PIRJOCS:/mnt/d/github/specification-tests$ ./run.sh nss.env

I have been unable to run the specification tests on https://localhost:8443

@bourgeoa
Copy link
Member

For information but may not impact the actual test-suite.

NSS has been updated to follow the spec on DELETE :

@bourgeoa
Copy link
Member

CI is now back running in NSS

I patched in the solid-crud-tests to remove await in rdflib.parse parameters
from

rdflib.parse(await result.text(), store2, resourceUrl, "text/turtle");

to

const resultText = await result.text()
rdflib.parse(resultText, store2, resourceUrl, "text/turtle");

CI is now back running in NSS

@melvincarvalho
Copy link
Contributor

@michielbdejong is this working for you now? Perhaps we can close this issue, if so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants